West Midlands | ITP-MAY25 | Saleh Yousef | Module-Data-Groups | Sprint-2 #622
Conversation
SalehOmar-Y
added
Needs Review
| @@ -11,5 +11,7 @@ const recipe = { | |||
| }; | |||
|
|
|||
| console.log(`${recipe.title} serves ${recipe.serves} | |||
| ingredients: | |||
| ${recipe}`); | |||
| ingredients:`); | |||
There was a problem hiding this comment.
Minor query here shouldn't the console read out "bruschetta serves 2, the ingredients are: ". Your code reads "bruschetta serves 2 ingredients: ". Otherwise this works
There was a problem hiding this comment.
code works however do some further research on how this could be improved. For example dealing with URL encoded string(decodeURIComponent). Then you could write a test to see if your solution works.
There was a problem hiding this comment.
Code works for tests. Do some further research on how code could be improved to handle edge cases such as undefined values.
fearcyf
added
Reviewed
| const [key, ...rest] = pair.split("="); | ||
| queryParams[key] = rest.join("="); |
There was a problem hiding this comment.
It looks like you fixed a bug here but didn't write a test for it - can you add a test for the edge case you fixed?
|
|
||
| test("parses querystring with multiple values", () => { | ||
| expect(parseQueryString("name=John&age=30")).toEqual({ | ||
| "name": "John", | ||
| "age": "30", | ||
| }); | ||
| }); | ||
| test("parses querystring with multiple values with same key", () => { | ||
| expect(parseQueryString("name=John&name=Jane")).toEqual({ | ||
| "name": "Jane", // Last value should overwrite previous ones | ||
| }); | ||
| }); | ||
|
|
||
| test("parses querystring without key", () => { |
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
This PR implements and tests functions: createLookup, parseQueryString, tally, and invert.
Includes bug fixes, edge case handling, and validation with unit tests.
Questions
Ask any questions you have for your reviewer.